Core: invoke ready watcher even if discovery fails - #719
Open
mnnekrashevich wants to merge 1 commit into
Open
Conversation
In async transport initialization (buildAsync) the ready watcher was never called when discovery failed: YdbDiscovery.waitReady throws IllegalStateException and the exception was swallowed by the executor, so the caller waited forever. Wrap waitReady in try/catch/finally so readyWatcher.run() is always executed, and log the discovery failure with its cause. Fixes ydb-platform#294
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Core: invoke ready watcher even if discovery fails
Fixes #294
Problem
When a transport is created asynchronously via
GrpcTransportBuilder.buildAsync(Runnable readyWatcher), thereadyWatchercallback is never invoked if the initial discovery fails. The caller waits forever and the transport is left in an unusable state without any notification.Root cause
YdbTransportImpl.startAsync(Runnable readyWatcher)schedules a task that callsdiscovery.waitReady(-1)and thenreadyWatcher.run():YdbDiscovery.waitReady()throwsIllegalStateException("Discovery failed", ...)when discovery fails. The exception is swallowed by the executor, soreadyWatcher.run()is never executed.Fix
Wrap
waitReadyintry/catch/finallyso thatreadyWatcher.run()is always executed, and log the discovery failure with its cause for diagnostics:The synchronous path (
build()) is not affected: there the exception is propagated to the caller, which is the correct behavior.Tests
Added
asyncBuildDiscoveryErrorTestinYdbTransportImplTest: it stubs the discovery call to fail withUNAVAILABLE, builds the transport withbuildAsync, runs the scheduler tasks and asserts that the ready watcher is invoked. The test fails without this fix (ready watcher is never called) and passes with it../mvnw -pl core test— 155 tests, all green